Support UDP in wsl-proxy #7618
Support UDP in wsl-proxy #7618
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Nino-K
force-pushed
the
add-udp-support-wsl-proxy
branch
3 times, most recently
from
5040319
to
6f2b119
Compare
| ) | ||
|
|
||
| const ( | ||
| defaultLogPath = "/var/log/wsl-proxy.log" | ||
| defaultSocket = "/run/wsl-proxy.sock" | ||
| bridgeIPAddr = "192.168.143.1" | ||
| // Set UDP buffer size to 8 MB | ||
| defaultUDPBufferSize = 8388608 // 8 MB in bytes |
There was a problem hiding this comment.
Would something like 8 * 1024 * 1024 be clearer?
| ) | ||
|
|
||
| func main() { | ||
| flag.BoolVar(&debug, "debug", false, "enable additional debugging.") | ||
| flag.StringVar(&logFile, "logfile", defaultLogPath, "path to the logfile for wsl-proxy process") | ||
| flag.StringVar(&socketFile, "socketFile", defaultSocket, "path to the .sock file for UNIX socket") | ||
| flag.StringVar(&upstreamAddr, "upstreamAddress", bridgeIPAddr, "IP address of the upstream server to forward to") | ||
| flag.IntVar(&udpBuffer, "udpBuffer", defaultUDPBufferSize, "max buffer size for the UDP socket io") |
There was a problem hiding this comment.
| flag.IntVar(&udpBuffer, "udpBuffer", defaultUDPBufferSize, "max buffer size for the UDP socket io") | |
| flag.IntVar(&udpBuffer, "udpBuffer", defaultUDPBufferSize, "max buffer size in bytes for UDP socket I/O") |
I started adding in bytes and then couldn't help myself LOL…
| if gvisorTypes.TransportProtocol(portProto.Proto()) == gvisorTypes.TCP { | ||
| p.handleTCP(portBindings, pm.Remove) | ||
| } | ||
| if gvisorTypes.TransportProtocol(portProto.Proto()) == gvisorTypes.UDP { |
There was a problem hiding this comment.
else if? Or even a switch(….Proto())?
| if remove { | ||
| p.udpConnMutex.Lock() | ||
| if udpConn, exist := p.activeUDPConns[port]; exist { | ||
| logrus.Debugf("closing UDPConn for port: %d", port) |
There was a problem hiding this comment.
Debugf with a mutex held seems kind of heavy?
There was a problem hiding this comment.
I agree, however, since it's only going to output when debugging is on, do you still think that would be an issue? But, regardless I'll move it.
There was a problem hiding this comment.
There's also a potential Errorf later. But yeah, it's probably fine.
| continue | ||
| } | ||
|
|
||
| p.udpConnMutex.Lock() |
There was a problem hiding this comment.
Do we need the mutex held for all of this, or just the one line after?
There was a problem hiding this comment.
Agreed, unnecessary defer.
| for { | ||
| b := make([]byte, p.config.UDPBufferSize) | ||
| n, addr, err := sourceConn.ReadFromUDP(b) | ||
| if err != nil { |
There was a problem hiding this comment.
Per PacketConn documentation:
Callers should always process the
n > 0bytes returned before considering the errorerr.
Nino-K
force-pushed
the
add-udp-support-wsl-proxy
branch
2 times, most recently
from
a499776
to
932e27b
Compare
The wsl-proxy previously only supported TCP connections. With this change, it now enables the listening and handling of UDP packets in the proxy. Signed-off-by: Nino Kodabande <[email protected]>
Signed-off-by: Nino Kodabande <[email protected]>
Nino-K
force-pushed
the
add-udp-support-wsl-proxy
branch
from
932e27b
to
3b49fde
Compare
| for { | ||
| udpMappings = portProxy.UDPPortMappings() | ||
| if len(udpMappings) > 0 { | ||
| close(ready) |
There was a problem hiding this comment.
I don't understand why this needs to be a separate goroutine, instead of just a normal loop, but 🤷
for len(portProxy.UDPPortMappings()) == 0 {
time.Sleep(100 * time.Millisecond)
}There was a problem hiding this comment.
You are correct, my original approach was polling for Port (before implementing UDPPortMappings) was doing things in a goroutine, but since we have the function, there is no need for a goroutine, removed.
Signed-off-by: Nino Kodabande <[email protected]>
Signed-off-by: Nino Kodabande <[email protected]>
Nino-K
force-pushed
the
add-udp-support-wsl-proxy
branch
from
3b49fde
to
6d9ab24
Compare
The wsl-proxy previously only supported TCP connections. With this change, it now enables the listening and handling of UDP packets in the proxy.
Fixes: #6821
Also, addresses: #7473 and #4873